Hi @fwyzard , @VinInn,
Should we implement the changes to the thresholds directly in the default values or is the use of a preprocessor flag fine as done in this PR?
Moreover, should I submit this PR also to the central CMSSW repository?

hi @stahlleiton ,
sorry for the delay, I should have checked the PR sooner.

The use of a preprocessor means that we cannot build the code for heavy ions in the Patatrack (or full CMSSW, eventually) release - we have either one version or an other.
This is fine if these changes are "just" for private validation and developments - the HI group can change the flag and rebuild the modules. However, at some point we need a more flexible solution.

Should we implement the changes to the thresholds directly in the default values or is the use of a preprocessor flag fine as done in this PR?

What (other?) changes do you mean ?

Moreover, should I submit this PR also to the central CMSSW repository?

This code is not in the main repository yet.

What (other?) changes do you mean ?

I was referring to the changes in this PR, whether the strategy used was fine.

So judging from your answer, the use of preprocessor flags are then not a good long term solution.
What other solutions do you propose?
I know at least that in the python configuration one can set up eras and modifiers for different run configurations, but I am not sure how to do that in the c++ code, without also changing the parameters for standard pp running.

There are two types of changes in the PR

• types, e.g.

  -  using hindex_type = uint16_t;
  +  using hindex_type = uint32_t;

• values, e.g.

  +#ifdef GPU_HION_EVENTS
  +    constexpr uint32_t nbytes_per_fed_max = 32 * 1024;
  +#else
       constexpr uint32_t nbytes_per_fed_max = 10 * 1024;
  +#endif

For the changes in types I can think of two approaches:

• if the change does not adversely impact the performance (i.e. reduce the throughput), we can just apply it
• if the change does impact does affect the performance, we can template the modules based on the type

For the changes in values, the more flexible approach would be to make them configurable at runtime (e.g. from some new python parameters).
Otherwise, they can be handled like the types, i.e. just be changes if they don't affect the performance, or made into template parameters otherwise.

If we go with the template approach, it would make sense to define just one "traits" struct with all the types and values for the 2-3 options, and template the individual modules based on just those.

we know ( see #474 ) that just the change of type impact throughput.
change in size may cause memory issues.

Said that, these kind of changes are likely to be needed for Phase2 as well.

I do not think we can afford to template on the hit index: it affects too many event-objects used also at high level (tracks).
We cannot template the whole cmssw.
One can try with mixed types (int32 for event objects, either int16 or int32 for objects used in algo), not very safe in my mind.

can we test this? @AdrianoDee
In standard wf as usual.

Validation summary

Reference release CMSSW_11_2_0_pre9 at 820b4c9
Development branch cms-patatrack/CMSSW_11_2_X_Patatrack at 586cac2

🚧 Validation running at fu-c2a02-37-01:/data/user/fwyzard/patatrack/validation/run_545.63OvryUj0S ...

@stahlleiton after merging #583 the #ifdef changes for hindex_type are no longer needed; do you think it would be possible to make all the remaining changes based on run-time configuration parameters rather than compile-time #defines or constants ?

not yet. there are a bunch of HistContainers that are templated with maxClusters (== maxHIts) and maxTracks
plus some containers for the CA and they are fixed at compile time

OK, so maybe for after the integration: would it make sense to make size the containers at construciton time instead of compile time ?

We can "choose" the container to use at runtime among a small (3? small,big,huge) set defined at compile time.

Closing this PR since changes have been implemented in CMSSW through:
cms-sw#34664
cms-sw#35017